Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features/error boundary #6

Merged
merged 4 commits into from
Mar 3, 2021
Merged

Features/error boundary #6

merged 4 commits into from
Mar 3, 2021

Conversation

ArkuVonSymfon
Copy link
Contributor

No description provided.

Copy link
Contributor

@tomaszantas tomaszantas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor things and we are ready to go ;)

const ErrorChild = <div className='custom-class'>{ErrorText}</div>

describe(`ErrorBoundary component test`, () => {
it('should component rendered properly with status [false]', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename: should render with status [false]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it after merge because there is more file which needs to be changed

const ErrorText = 'Error message'
const ErrorChild = <div className='custom-class'>{ErrorText}</div>

describe(`ErrorBoundary component test`, () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename: Component:ErrorBoundary. Then we can use the same pattern with other tests, like:

Service:AuthService
Utilities:Validation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it after merge because there is more file which needs to be changed

<h1>{ErrorText}</h1>
</ErrorBoundary>,
)
expect(rendered).toBeDefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try to find a h1 element to make the test a bit more accurate.

For example:

const { container } = render(
      <ErrorBoundary showError={true}>
        <h1>{ErrorText}</h1>
      </ErrorBoundary>,
    )
    
    // Find by selector:
    const h1Element = container.querySelector('h1')
    expect(h1Element).toBeTruthy()
    
    // ...or find by text:
    const h1Element = container.getByText(ErrorText)
    expect(h1Element).toBeTruthy()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it after merge because there is more file which needs to be changed

import { render } from '@testing-library/react'
import { ErrorBoundary } from '../../components/UI'

const ErrorText = 'Error message'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The capitalized names should be used with components, containers, services or models names.

For variables, use: errorText and errorChild.

The snake case, use with configurable variables, ie. API_KEY=xxxx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it after merge because there is more file which needs to be changed

@ArkuVonSymfon ArkuVonSymfon merged commit a441515 into master Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants